feat(runetime-host): sign in to GitHub Copilot with a device grant - #3390
feat(runetime-host): sign in to GitHub Copilot with a device grant#3390hgaol wants to merge 26 commits into
Conversation
c7d8924 to
4f133f1
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for filling the sign-in gap described in #3388. The problem is valid, and the provider-specific RFC 8628 parsing is carefully bounded.
The remaining implementation concern is ownership: the device flow currently runs beside the existing Host OAuth authority, duplicating the attempt lifecycle and producing the race described inline. The simpler final state appears to be extending the existing Host OAuth provider slice for GitHub Copilot and keeping Desktop responsible only for importing credentials that genuinely depend on the local gh installation.
I also left two independent inline notes about token lifetime and the OAuth app identity.
AI-assisted review disclosure: Codex performed exact-head analysis, with independent reviewer passes focused on lifecycle, concurrency, security, compatibility, and OAuth protocol behavior. The findings were revalidated against head 4f133f11b4442ed572379da7040965e9a7699ca2.
|
PR need a rebase before reviewing and approve~ |
GitHub Copilot is the only registered oauth_token provider without a login flow. Desktop's connectExistingLogin only scavenges COPILOT_GITHUB_TOKEN, GH_TOKEN, GITHUB_TOKEN, or `gh auth token`, rejects classic PATs, and needs a credential carrying Copilot Requests — a permission plain `gh auth login` does not grant. Users who own a subscription still have no way to sign in. Add the RFC 8628 device authorization the provider was missing, alongside the existing Codex and xAI enrollments. The grant yields the same gho_/ghu_ account token createGitHubCopilotAccountTokens already accepts, so only credential acquisition changes. GitHub reports authorization_pending and slow_down as HTTP 200 with an error body rather than a 4xx, so the error code is read before the success shape. The verification URL is pinned to github.com because the presentation layer opens it in the user's browser, and only read:user is requested, so the grant cannot reach a user's code. The provider already sends the vscode Copilot Chat editor headers (GITHUB_COPILOT_COMPAT_HEADERS), so the client id presented here does not change who Maka claims to be. Fixes apache#3388 Generated-by: GitHub Copilot CLI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Wire the device enrollment into Settings so a user with a Copilot subscription can sign in, instead of first minting a fine-grained PAT with Copilot Requests or having a `gh` login that happens to carry it. `beginDeviceLogin` asks GitHub for a code and `completeDeviceLogin` waits for the user to authorize it; both ride the existing direct-account login flow, so the pending guard, snapshot refresh, and unmount safety stay in one place. The pending grant lives in the main process, so closing Settings cannot strand a poll holding a credential, and the secret still reaches only the Host vault. Importing an existing `gh` credential remains available as a secondary action for machines that are already authenticated, and both paths now adopt an account through one place that refuses to store a credential before the account proves it returns usable models. The verification code is shown as text rather than opening a browser: the renderer has no general openExternal bridge by design, and the device URL is pinned to github.com by the enrollment. Fixes apache#3388 Generated-by: GitHub Copilot CLI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Route the GitHub Copilot device grant through `HostOAuthCoordinator` instead of a second login-session authority in Desktop. Assigning `deviceLogin` only after the begin request resolved let a reopened Settings pane or a second IPC client overlap attempts, so an older completion could replace the credential a newer login had just committed. `github-copilot` joins `OAUTH_LOGIN_PROVIDERS` and the interactive login providers, and the coordinator grows a provider slice beside xAI and Codex. It therefore inherits one serialized start, supersede and cancellation boundaries, Host residency across polling, the configured network transport, and an atomic credential commit. Desktop keeps only the local `gh` credential import; account state, refresh, and sign-out ride the shared channels the coordinator's adapter already registers. Older peers cannot start a login for the new provider, so the compatibility epoch moves to 32. Preserve the lifetime GitHub returns. Every account token was recorded with `expires_at = Number.MAX_SAFE_INTEGER` while the refresh path returned it unchanged, so an expiring token would stop working at expiry and require another interactive login. The enrollment now carries `expires_in` and `refresh_token` through, refresh performs the refresh grant for an expiring record, and an expiring response with no refresh token is rejected rather than stored. A token with no declared lifetime keeps the sentinel and still refreshes without provider I/O. Record the OAuth app identity. The client id is GitHub's Copilot editor app: GitHub's consent screen names that application while Maka receives and stores the credential, and no published authorization for third-party reuse exists. Provenance, consent identity, and the open authorization question are written down in `docs/github-copilot-oauth-identity.md`, and the device flow is now opt-in behind `MAKA_GITHUB_COPILOT_DEVICE_LOGIN_EXPERIMENTAL=1` so it is not a public sign-in path before that basis is established. Importing a credential the user already holds stays the shipped route. Generated-by: Claude Code
The device flow was gated opt-in, which left Settings with no sign-in button at all — only the local credential import. Recording the app identity is what the review asked for; hiding the feature was not. Enrollment now ships on with a kill switch, the same shape Codex already uses: `MAKA_GITHUB_COPILOT_DEVICE_LOGIN_EXPERIMENTAL=0` refuses the login at the Host. The panel stops probing that flag and always renders the button, because the Host owns the decision and answers a disabled start with `experimental_disabled`; a second copy of the gate in the renderer could only disagree with it. The consent identity question in `docs/github-copilot-oauth-identity.md` is unchanged and still open. Generated-by: Claude Code
a3dcc93 to
16f6520
Compare
Rebased |
…-device-flow-login # Conflicts: # packages/runtime-host/src/__tests__/protocol.test.ts # packages/runtime-host/src/protocol/index.ts
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for moving GitHub Copilot enrollment onto HostOAuthCoordinator. That resolves the original lifecycle and stale-completion problem, and the token lifetime/refresh work is now on the right authority.
I do not think another round of isolated patches would be fair or productive here. Could we converge on one final state instead?
[P1] Shipping boundary: the current provenance document explicitly says the authorization basis for reusing the Copilot editor OAuth identity is not established, while the feature is enabled by default. Until GitHub provides a public compatibility basis or a Maka-authorized app identity, this path should be explicit opt-in/default-off. The kill switch can remain for operations, but it cannot serve as the authorization basis.
[P2] Account adoption: device authorization should not commit a credential until the GitHub provider slice has verified that the account exposes at least one usable Copilot model. That validation belongs before the coordinator’s atomic commit, using the same Host transport; the Desktop post-login synchronization should not be responsible for deciding whether authentication succeeded.
[P2] Refresh completion: GitHub Copilot should join the existing single-401 refresh/replay seam. An access token can be rejected before its local expires_at; in that case the Host should force-refresh once, CAS-persist the new generation, replace the Authorization header, and replay the request once.
I would also like this revision to remove the Desktop lifecycle that the Host path has replaced, rather than adding the fixes on top:
- fold
GitHubCopilotSubscriptionServiceinto a stateless “import existing local credential” adapter; - remove the unused
directmode fromuseOAuthLoginFlow; - route connection-detail re-login through the shared Host OAuth service;
- keep local
ghimport only as a secondary provider-catalog action; - remove the unused
isExperimentalEnabledpreload/IPC query surface.
The provider-specific device parsing and its security/error tests should stay. The goal is not to make OAuth artificially tiny; it is to leave one lifecycle authority and remove the roughly 280–330 lines whose responsibilities have moved to the Host.
A clean final shape would be: Runtime owns the GitHub device codec, Host owns attempt/entitlement/credential/refresh/401 recovery, and Desktop owns only generic presentation plus local credential discovery. Once that state is in one exact head with epoch 35 and focused entitlement/401 regressions, we can review the final result as a whole instead of asking for another sequence of patches.
AI-assisted review disclosure: OpenAI Codex coordinated independent exact-head reviews of the OAuth lifecycle, consent/provenance boundary, refresh behavior, and simplification opportunities. I reviewed and approved this final-state recommendation before posting.
Leave one lifecycle authority for Copilot accounts. The Host owns the attempt, the entitlement check, the credential, refresh, and 401 recovery; Desktop keeps only generic presentation and discovery of a credential this machine already holds. The device sign-in ships off. Maka has no published authorization for the editor OAuth identity the grant presents, so an install opts in explicitly with `MAKA_GITHUB_COPILOT_DEVICE_LOGIN_EXPERIMENTAL=1`. This reverses the default chosen in "restore the GitHub Copilot sign-in in Settings": the flag records an operator accepting the consent mismatch for their own install, and cannot stand in for a basis that does not exist. A GitHub account is not a Copilot subscription. The provider slice now asks the Copilot API what the authorized account reaches, on the Host transport, before the coordinator's atomic commit; an account with no usable model fails the attempt as `provider_rejected` instead of committing a credential that breaks on first use. Copilot joins the single-401 refresh seam: force-refresh once, CAS-persist the new generation, replace the Authorization header, replay once. A record GitHub declared no lifetime for resolves to the token just rejected, so that replay is skipped rather than spent. Removed with their responsibilities: the Desktop subscription service, the `direct` mode in the login controller, the Copilot-specific re-login notice, and the unused `isExperimentalEnabled` query surface. Generated-by: Claude Code
An account Connection is created before anyone can ask the account what it has — the OAuth login path holds no credential at that point — so its enabled ids start as the provider's curated fallback list. Discovery then ran without ever reconciling them, leaving every guessed id selectable, testable, and sendable on an account that may never have exposed it. GitHub Copilot surfaced this: a connection test picks the first enabled id, the Copilot models endpoint does not list it, and the test fails on a model the user never chose while chat works on one they did. The local credential import was unaffected because it passes discovered ids at creation, so only the shared OAuth path carried the gap. Once a fetched inventory exists it is the authority: ids it does not list are dropped. Ids are never added, so a narrower selection the user made survives, and the discovered list is adopted whole only when nothing they kept remains. Generated-by: Claude Code
The notice opened the provider's device page and showed nothing else, but that page accepts nothing until the user types the one-time code the Host received. Re-login was therefore a button that led to a dead end. The code now appears beside the notice's own copy, for the flows whose device page asks for it. xAI is excluded because its verification URL carries the code itself, which is why the catalog panel has always hidden it there too. Generated-by: Claude Code
hgaol
changed the title
…b-copilot-device-flow-login # Conflicts: # packages/runtime-host/src/__tests__/protocol.test.ts # packages/runtime-host/src/protocol/index.ts
An account Connection is created before anyone can ask the account what it has — the OAuth login path holds no credential at that point — so its enabled ids start as the provider's curated fallback list, in the order this build ships them. Taking the first of those as the account's initial default names a model the account may never serve, which is how a GitHub Copilot connection test came to fail on a model the user never chose while chat worked on one they did. The live response now picks the order that first default is chosen from. It does not pick the set: an id the response omitted stays enabled, because a `/models` answer that cannot see a model is not evidence the account cannot run it — the rule `authorizeConnectionModel` states for every other reader of an inventory. This replaces the pruning added in "adopt the account's own model inventory after sign-in", which reached the same connection test by treating the live list as an allowlist over ids only the user writes. Generated-by: Claude Code
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for taking this through to a much cleaner final state. I re-reviewed exact head b5da959.
The original blocking concerns are resolved: interactive enrollment now runs through HostOAuthCoordinator, Copilot entitlement is verified before the atomic credential commit, rejected access tokens join the shared single-401 refresh/replay path, and the borrowed OAuth identity is default-off behind an explicit per-install opt-in. The replaced Desktop lifecycle, direct login mode, special re-login path, and unused experimental query have also been removed.
I left two non-blocking P2 comments. They are two remaining expressions of the same ownership invariant: the selected Runtime Host should be the sole authority for enrollment availability, entitlement, and credential adoption, while Desktop should only present the flow and discover credentials that exist locally. Neither warrants another broad redesign or a Request Changes review; the clean final direction is described inline.
The provenance document correctly states that authorization to reuse the editor OAuth identity has not been established. This approval accepts the dormant, explicit-opt-in shipping boundary for this change; it does not assert or create authorization from GitHub.
AI-assisted review disclosure: OpenAI Codex coordinated independent exact-head reviews of OAuth lifecycle and concurrency, identity and shipping boundaries, refresh behavior, remote-Host compatibility, and simplification. I verified the final diff, relevant source paths, prior findings, current head, CI, review threads, and the approval recommendation before posting.
| * Desktop owns exactly one thing for GitHub Copilot: importing a credential | ||
| * that already exists on this machine (`gh` / a compatible PAT). Interactive | ||
| * enrollment is not here — the device grant runs through the Host's OAuth | ||
| * coordinator like every other account login, so there is one authority that |
There was a problem hiding this comment.
[P2] Keep local credential import discovery-only
Reachability: ② reasonable concurrent or multi-client path. The local adapter currently performs entitlement discovery in Desktop, then this handler reads the Host's current credential revision and commits the imported secret. If import A is waiting on model discovery while an interactive login B commits through HostOAuthCoordinator, A can resume, adopt B's latest revision as its expected value, and overwrite the newer login. A remote Host also validates the import through the Desktop network environment rather than the Host transport.
The clean final state is for Desktop to return only the locally discovered raw credential and for a Host adoption operation to own entitlement, generation/supersede, and the atomic credential commit alongside interactive login. That closes the race and preserves one network and credential authority without adding another Desktop guard.
resolved. |
…b-copilot-device-flow-login # Conflicts: # apps/desktop/src/renderer/settings/provider-connection-detail.tsx # apps/desktop/src/renderer/settings/use-oauth-login-flow.ts
The IPC adapter read this Desktop process's environment before calling the Host. Against a remote Runtime Host that is the wrong authority: a Host whose operator enabled GitHub Copilot enrollment was still refused whenever the Desktop process happened not to set the same variable, and no local answer could have been right, because the environment that governs enrollment is the Host's. The start now always reaches the Host, and its `operation_unavailable` refusal is what the renderer reports — kept as `experimental_disabled` so the user is told the enrollment is off rather than that authorization failed. Generated-by: Claude Code
…t-device-flow-login
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for moving the interactive device flow onto HostOAuthCoordinator; the previous stale-completion root cause is now closed, and token lifetime, refresh, entitlement-before-commit, supersede, and atomic credential commit all follow the existing Host seam. The local-import authority thread remains open, and I found one additional recoverability issue in entitlement classification inline.
AI-assisted review disclosure: OpenAI Codex coordinated three independent exact-head review passes. I verified the retained lifecycle and error-classification paths, existing threads, current-head checks, mergeability, and review state, and I made the final review decision.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks — one final cancellation edge surfaced in the Copilot 401 replay path. It is bounded and does not corrupt the credential, so I am classifying it as P3 rather than another merge-blocking P2; the two existing P2 items remain the actual readiness gate.
AI-assisted review disclosure: OpenAI Codex delegated an independent protocol and cancellation review. I verified the retained path and severity against this exact head.
…able The entitlement gate turned every `/models` failure into "this account has no Copilot". A timeout, a dropped connection, a 429, a 5xx, or an unreadable body told a paying subscriber they were ineligible and sent them back through a device login that was never the problem — while no credential had been committed and the next attempt would usually have worked. Only a proven refusal is an entitlement answer now: a catalog the account could read that lists nothing, or a deterministic 401/403. Everything else keeps the provider status and surfaces as an authorization failure the user simply retries. The shared 401 replay seam also gains the request's own signal. A caller who cancels while the forced refresh is in flight is released with its cancellation instead of waiting out the refresh timeout and then receiving the stale 401 as though the credential had been rejected; the refresh itself is left to settle, because a spent grant is worth persisting even once nobody wants the replay. Generated-by: Claude Code
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks — reviewed fresh against this head rather than relying on the earlier approval, since that one predates real work on the branch.
What convinced me, in the order I checked it:
The problem is framed correctly. A device authorization grant proves a GitHub account, and a GitHub account is not a Copilot subscription. Calling /models before completeInteractiveOAuthLogin and only committing credentials when reachable models come back is the distinction that keeps a successful sign-in from being mistaken for an entitlement. Conflating those two is the usual way this feature goes wrong, and it is handled here.
The RFC 8628 polling loop covers the cases that matter. authorization_pending, slow_down, access_denied and expired_token all arrive as HTTP 200 and are handled separately, with unknown errors terminating rather than spinning. The loop sleeps by interval first and stops against local expiry rather than trusting the server to say stop. The detail I appreciated most: each token request uses its own signal, because once the grant may already have been consumed, a cancellation that discards the response throws away a credential the user really did authorize — while the sleep and retry boundaries stay cancellable. That distinction is easy to miss.
verification_uri is validated before it is opened: HTTPS only, github.com or a subdomain, no embedded credentials. Handing a server-supplied URL to the system browser unchecked is the classic hole here, and it is closed.
Credentials do not get a private path. Device code, user code, expiry and interval stay in Host memory and never reach the Desktop capability, which only receives the URL and the display user code. Storage goes through the existing canonical vault with the shared refresh/CAS seam, and a GitHub token without a lifetime uses an explicit non-expiring sentinel instead of fabricating a refresh grant. The 401 path reuses the shared refresh-and-replay rather than adding a second one.
On scope, the change removes the duplicated device flow, polling and persistence that lived on the Desktop side instead of adding a parallel implementation, so the authority for this ends up in one place.
On the default being off: gating interactive login behind MAKA_GITHUB_COPILOT_DEVICE_LOGIN_EXPERIMENTAL=1 looks right to me rather than over-cautious. The device flow reuses the GitHub editor OAuth identity, so the party that gave consent and the party holding the credential are not the same — leaving the existing gh/PAT import as the default path while that is true is the honest position, and it is the kind of thing worth stating in the code rather than deciding silently.
No P0–P3 findings. test and audit are both terminal green on this exact head.
Review assisted by AI (Claude Opus 5 and GPT-5.6, reviewed independently). Findings were verified against the files at this head; the reviewers are accountable for them.
…-device-flow-login # Conflicts: # apps/desktop/src/main/__tests__/github-copilot-subscription-service.test.ts # apps/desktop/src/main/oauth/github-copilot-subscription-service.ts # apps/desktop/src/main/runtime-host-github-copilot-ipc-main.ts # packages/runtime-host/src/__tests__/protocol.test.ts # packages/runtime-host/src/protocol/index.ts # packages/runtime/src/oauth-provider-contracts.ts
|
Hi — this PR conflicts with current I tested a rebase onto current
These are real source conflicts, so they need your judgement rather than a mechanical rebase — please rebase onto current Thanks for the contribution — happy to help if any conflict is unclear. AI-assisted maintenance note, not a review. It does not count as the required human review under |
…-device-flow-login # Conflicts: # packages/runtime-host/src/__tests__/protocol.test.ts # packages/runtime-host/src/protocol/index.ts
|
Rebased onto current main — resolved as a merge rather than a rebase to preserve your existing approval, and since the repo squash-merges the branch history won't land on main either way. Happy to redo it as a true rebase if you'd prefer. main had moved again by the time I got to it, so this is against 6b48ed3. Two conflicts, both in packages/runtime-host: the compatibility epoch (this branch held 40, which upstream has since taken for the message-queue mutation ops — Copilot is now 43, and scripts/protocol-epoch-check.mjs passes), and protocol.test.ts, where I kept your new credential-rotation and provider-capacity tests and re-anchored the Copilot assertion to > 42. I also ran npm run write:asf-headers for the five files this branch added before the header gate existed. CI shows one failure: host-kernel.test.js "slow domain work preserves multiplexed requests" timed out on goal.query. That file is untouched by this branch and passes locally (506ms); given a2f2a1a it looks like the same flaky area. Could you re-run the job? I don't have permission to. |
…its gate `slow domain work preserves multiplexed requests and retires only explicit deadlines` issues a `goal.query` with a 50ms deadline, then awaits the handler-entered gate before calling `assert.rejects` on it. The deadline is shorter than that gate can take to open on a loaded runner, so the timeout could reject while no handler was attached yet. Node then reports it as an unhandled rejection and fails the test on timing rather than on behaviour. Attach the rejection assertion at request time and await it after the gate. The request still always times out, so the assertion keeps its teeth.
…-device-flow-login # Conflicts: # packages/runtime-host/src/__tests__/protocol.test.ts # packages/runtime-host/src/protocol/index.ts
M4n5ter
left a comment
M4n5ter
left a comment
There was a problem hiding this comment.
English
Reviewed exact head 0685beb21aada31cfedde4ab92a18a8c10ebb03a: NO-GO — 1×P1 remains open in the existing OAuth-identity thread.
The Host consolidation is a real simplification: it removes the Desktop-owned Copilot credential lifecycle and routes enrollment, refresh, recovery, and logout through one Runtime Host authority. The retained local-import adapter is appropriately local-only. I found no additional decision-changing correctness issue in those paths.
Verification: 188/188 focused affected tests passed; Core, Storage, Runtime, and Runtime Host builds passed; Biome and git diff --check passed. This head currently has 0 exact-head check-runs and conflicts with current main; full UI/Desktop integration is also stale against current cross-package APIs. Those integration gates therefore remain unproven independently of the P1.
中文
已审查 exact head 0685beb21aada31cfedde4ab92a18a8c10ebb03a：NO-GO——现有 OAuth 身份讨论中的 1 条 P1 仍未关闭。
Host 收口确实降低了复杂度：它删除了 Desktop 自己维护的 Copilot 凭证生命周期，把登录、刷新、故障恢复和退出统一到 Runtime Host 权威。保留的本地导入适配器也正确地限定在本机能力边界内。这些路径上没有发现其他足以改变合并判断的问题。
验证结果：受影响的 focused tests 188/188 通过；Core、Storage、Runtime、Runtime Host 构建通过；Biome 与 git diff --check 通过。当前 head 的 exact-head check-runs 为 0，且与当前 main 冲突；完整 UI/Desktop 集成也因跨包 API 已过期而无法成立。因此，即使不考虑该 P1，集成门禁也仍未得到证明。
|
Heads-up on why this PR has no CI results: it is not waiting for someone to release a run. There are zero workflow runs on the current head, because the PR is currently conflicting with That makes it look identical to "checks not applicable," which it is not. Rebasing onto current Flagging this because a review cannot be closed out against a head that has no gate evidence. |
Dismissing a stale approval. This is bound to an old head; the PR has since advanced, is currently conflicting with main, and has a live [P1] outstanding. Because this repository does not dismiss stale reviews automatically, leaving the approval in place would let the PR become mergeable again the moment the conflict is resolved and CI goes green, with that finding still open. Re-approval should happen at whatever head carries the fix.
There was a problem hiding this comment.
English
Incrementally reviewed exact head 0b9ebaf938178cf640309d44064021a8eaa1500c: NO-GO — 2×P1; no lower-severity finding.
- The previously reported borrowed OAuth application-identity boundary remains open. The new merge commit leaves the OAuth contract, identity document, and Host enrollment path byte-identical to the prior reviewed head.
- This head and current
mainindependently assign different incompatible protocol meanings to epoch 44. The current-main peer and this head can therefore pass the compatibility handshake and fail at the firstgithub-copilotlogin request. This is not eligible for a compatible-change declaration because the old closed decoder rejects the new provider.
The Host consolidation remains a genuine simplification: it removes the Desktop-owned credential lifecycle and preserves one Runtime Host authority. The upstream merge introduces no new owner or state path beyond the two findings above.
Verification on this head: Core, Storage, MCP, Runtime, Runtime Host, Computer Use, Desktop main, and Desktop preload built; 253/253 focused affected tests passed; Biome checked 37 changed source/config files and git diff --check passed. The pre-existing full UI build still fails on stale cross-package API shapes.
Gate evidence is unavailable, not merely pending: the PR still conflicts with current main, so GitHub cannot synthesize a merge ref and no pull_request workflow/check-run exists (total_count=0). A rebase must resolve the epoch collision and make the merge result available before CI can provide any gate evidence.
中文
已增量审查 exact head 0b9ebaf938178cf640309d44064021a8eaa1500c：NO-GO——2 条 P1；没有更低级别 finding。
- 上一轮报告的借用 OAuth 应用身份边界仍未关闭。新 merge commit 没有改变 OAuth 契约、身份说明文档或 Host 登录路径，它们与上一轮已审 head 字节一致。
- 当前 head 与当前
main独立地给 epoch 44 分配了两种不同且不兼容的协议语义。因此 current-main peer 与本 head 可以通过兼容性握手，却在第一次github-copilot登录请求时失败。这里不能使用 compatible-change 声明，因为旧的闭合解码器确实会拒绝新 provider。
Host 收口仍是真正的简化：它删除了 Desktop 自己维护的凭证生命周期，并保留单一 Runtime Host 权威。这次 upstream merge 没有在上述两条 finding 之外引入新的 owner 或 state path。
当前 head 的验证结果：Core、Storage、MCP、Runtime、Runtime Host、Computer Use、Desktop main 与 Desktop preload 构建通过；受影响的 focused tests 253/253 通过；Biome 检查 37 个变更源码/配置文件通过，git diff --check 通过。既有的完整 UI 构建仍因过期的跨包 API 形状失败。
门禁证据是无法取得，不是普通的等待中：PR 仍与当前 main 冲突，GitHub 无法生成 merge ref，因此不存在 pull_request workflow/check-run（total_count=0）。必须先 rebase、解决 epoch 冲突并让 merge result 可生成，CI 才能提供任何门禁证据。
| // Increment when the same protocol version no longer guarantees safe Client-Host | ||
| // interoperability. Mismatches are rejected before domain commands are admitted. | ||
| export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 43 as const; | ||
| export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 44 as const; |
There was a problem hiding this comment.
English
[P1] Give this incompatible wire shape an epoch strictly beyond current main.
This exact head assigns epoch 44 to adding github-copilot to the closed OAUTH_LOGIN_PROVIDERS decoder, while current main independently assigns epoch 44 to retiring the Session last-used timestamp. A current-main peer and this head therefore advertise the same epoch and pass the compatibility handshake even though the current-main Host rejects a github-copilot login start at the strict provider decoder. The first such request fails mid-session instead of the incompatible pair being refused during handshake.
This is the same-number sibling-branch collision the merge-result guard is designed to reject. Evaluating the guard with base=44, head=44, and the changed OAuth protocol file returns failure; the compatible-change declaration is not valid here because an old peer does reject the new shape. Please rebase onto current main, read its epoch at that time, and assign this incompatible change a strictly greater value (with the corresponding floor test/comment). Do not preselect a fixed number before rebase.
中文
[P1] 请为这项不兼容 wire shape 分配一个严格大于当前 main 的 epoch。
当前 exact head 把 epoch 44 用于向闭合的 OAUTH_LOGIN_PROVIDERS 解码器加入 github-copilot；当前 main 又独立地把 epoch 44 用于移除 Session 最后使用时间。两边因此会声明相同 epoch 并通过兼容性握手，但 current-main Host 会在严格 provider 解码器处拒绝 github-copilot 登录请求。结果是第一次此类请求在 session 中途失败，而不是在握手阶段拒绝这对不兼容 peer。
这正是 merge-result guard 要阻止的“同号 sibling branch”冲突。以 base=44、head=44 和变更的 OAuth protocol 文件执行 guard 会得到失败；这里也不能使用 compatible-change 声明，因为旧 peer 确实会拒绝新形状。请先 rebase 到当前 main，读取当时的 epoch，再为这项不兼容变更分配严格更大的值，并同步更新 floor test/comment。不要在 rebase 前预先认领固定号码。
There was a problem hiding this comment.
Already satisfied by the state pushed, and confirmed by the merge just landed (85abcaae7).
Main is at 47 (the merge you're revalidating against), the branch is at 48, with the floor comment naming the reason. I ran your own criterion — the merge-result guard — against current main: base=47, head=48, pass. The 44-vs-44 collision this thread describes was against an older head; it's stale as of this one.
…-device-flow-login # Conflicts: # packages/runtime-host/src/__tests__/protocol.test.ts # packages/runtime-host/src/protocol/index.ts
…-device-flow-login # Conflicts: # apps/desktop/src/renderer/settings/provider-connection-detail.tsx # apps/desktop/src/renderer/settings/provider-oauth-section.tsx # packages/runtime-host/src/protocol/index.ts
…b-copilot-device-flow-login
|
Understood, thanks for the explanation. That's resolved now: the merge landed at 85abcaa, the tree is clean against current main (8801edc), and workflow runs are live on this head (audit, test). Will report back once they're green — the one thing I'm watching is host-kernel.test.js's "slow domain work preserves multiplexed requests," which timed out on the previous CI attempt but is untouched by this branch and passes locally; likely the same flaky area as a2f2a1a. |
…-device-flow-login # Conflicts: # apps/desktop/src/main/oauth/github-copilot-subscription-service.ts # packages/runtime-host/src/__tests__/protocol.test.ts # packages/runtime-host/src/protocol/index.ts
|
Heads-up on a number collision, not a review of the change itself. This branch declares Rather than a number, the reliable step is to re-derive it at rebase time, because other branches are moving too: Take the first value above 简体中文提醒一个编号冲突，不是对改动本身的评审。 这个分支声明了 与其给一个数字，更可靠的做法是在 rebase 时重新推导一次，因为其它分支也在移动： 取比 |
…b-copilot-device-flow-login
|
Re-derived rather than reused: 49 turned out to be independently claimed by two other open PRs (#3299, #3651), not just main. Moved to 50, which no open PR currently claims, and matched the floor test's assertion (already |
3 participants
Summary
GitHub Copilot was the only registered
oauth_tokenprovider without a real sign-in path. Desktop only scavenged an existingCOPILOT_GITHUB_TOKEN/GH_TOKEN/GITHUB_TOKEN/gh auth token, rejected classic PATs, and left users minting a fine-grained PAT with Copilot Requests.This PR adds the missing RFC 8628 device-flow enrollment in
@maka/runtimeand wires it into Desktop Settings:beginDeviceLoginrequests a user code;completeDeviceLoginpolls until authorizedghimport adopt an account only after it proves usable modelsopenExternalbridge); enrollment pins the URL to github.com and requests onlyread:userImporting an existing
ghcredential remains available as a secondary action.Fixes #3388
Verification
lint/typecheck/ workspace suites not re-run in this PR-opening step; rely on CItestcheckScreenshot
Sign in
Successful GitHub Copilot subscription account import after sign-in:
AI use
Select exactly one:
Tool(s) and scope: GitHub Copilot CLI assisted implementation of the device-flow enrollment, Desktop wiring, tests, commit message cleanup, and PR creation. The human contributor remains responsible for the change.
Checklist
Does this PR entail a change in behavior?